Implement filter_cookies() with domain-matching and path-matching #7944

xiangxli · 2023-12-04T13:09:48Z

What do these changes do?

Implement filter_cookies() with domain-matching and path-matching on the keys, instead of testing every single cookie.

Fork from the below PR, force push caused it to be closed.
#7777

Are there changes in behavior for the user?

no

Related issue number

#7583

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bugfix)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of the following:
  - .feature: Signifying a new feature.
  - .bugfix: Signifying a bug fix.
  - .doc: Signifying a documentation improvement.
  - .removal: Signifying a deprecation or removal of public API.
  - .misc: A ticket has been closed, but it is not of interest to users.
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

xiangxli · 2023-12-04T13:10:47Z

@Dreamsorcerer Hey buddy. The PR 7777 was closed by force push, creating this new PR to continue.

for more information, see https://pre-commit.ci

tests/test_cookiejar.py

aiohttp/cookiejar.py

Co-authored-by: Sam Bull <[email protected]>

for more information, see https://pre-commit.ci

tests/test_cookiejar.py

aiohttp/cookiejar.py

Co-authored-by: Sam Bull <[email protected]>

for more information, see https://pre-commit.ci

aiohttp/cookiejar.py

tests/test_cookiejar.py

CHANGES/7583.feature

codecov · 2024-01-20T15:49:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2670e7b) 97.42% compared to head (ac55d64) 97.49%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7944      +/-   ##
==========================================
+ Coverage   97.42%   97.49%   +0.07%     
==========================================
  Files         107      107              
  Lines       32601    32587      -14     
  Branches     3799     3799              
==========================================
+ Hits        31760    31770      +10     
+ Misses        634      614      -20     
+ Partials      207      203       -4

Flag	Coverage Δ
CI-GHA	`97.41% <100.00%> (+0.06%)`	⬆️
OS-Linux	`97.07% <100.00%> (+0.13%)`	⬆️
OS-Windows	`95.57% <100.00%> (+0.04%)`	⬆️
OS-macOS	`96.70% <100.00%> (-0.18%)`	⬇️
Py-3.10.11	`95.50% <100.00%> (?)`
Py-3.10.13	`96.88% <100.00%> (-0.01%)`	⬇️
Py-3.11.7	`96.53% <100.00%> (+0.11%)`	⬆️
Py-3.12.1	`96.66% <100.00%> (?)`
Py-3.8.10	`95.46% <100.00%> (-0.01%)`	⬇️
Py-3.8.18	`96.81% <100.00%> (-0.01%)`	⬇️
Py-3.9.13	`95.46% <100.00%> (-0.01%)`	⬇️
Py-3.9.18	`96.85% <100.00%> (+<0.01%)`	⬆️
Py-pypy7.3.15	`96.40% <100.00%> (?)`
VM-macos	`96.70% <100.00%> (-0.18%)`	⬇️
VM-ubuntu	`97.07% <100.00%> (+0.13%)`	⬆️
VM-windows	`95.57% <100.00%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

for more information, see https://pre-commit.ci

Dreamsorcerer · 2024-01-20T18:23:38Z

@bdraco Made a few more optimisations after looking at codecov. Would be good if you can double check them.

bdraco · 2024-01-20T18:28:19Z

Starting my day soon, I'll be at my desk in about an hour run through everything

bdraco · 2024-01-20T19:45:31Z

This looks good. I'll put it on production

bdraco · 2024-01-20T20:00:48Z

Nothing obvious broke on production.

Will do a py-spy to look at the flow ...

bdraco · 2024-01-20T20:04:21Z

Looks like py-spy won't work here as its too fast now to get any samples compared to the rest of the program.

Will do cProfile instead

bdraco · 2024-01-20T20:10:50Z

I think this is good to go.

patchback · 2024-01-21T00:05:08Z

Backport to 3.10: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 54ceb6f on top of patchback/backports/3.10/54ceb6fd48ff4c4add2107a460376fc72c2b76e9/pr-7944

Backporting merged PR #7944 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.10/54ceb6fd48ff4c4add2107a460376fc72c2b76e9/pr-7944 upstream/3.10

Now, cherry-pick PR Implement filter_cookies() with domain-matching and path-matching #7944 contents into that branch:
```
$ git cherry-pick -x 54ceb6fd48ff4c4add2107a460376fc72c2b76e9
```
If it'll yell at you with something like fatal: Commit 54ceb6fd48ff4c4add2107a460376fc72c2b76e9 is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 54ceb6fd48ff4c4add2107a460376fc72c2b76e9
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Implement filter_cookies() with domain-matching and path-matching #7944 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.10/54ceb6fd48ff4c4add2107a460376fc72c2b76e9/pr-7944

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

) --------- Co-authored-by: Sam Bull <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> (cherry picked from commit 54ceb6f)

) (#7944) --------- Co-authored-by: Sam Bull <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> (cherry picked from commit 54ceb6f) --------- Co-authored-by: xiangxli <[email protected]>

…_cookies (#8537) **This is a backport of PR #8535 as merged into master (7108d64).**  ## What do these changes do? Small speed up to cookiejar Using `str.format` is ~16% faster than the lambda followup to #7944 (comment). I was hoping to use `join` there but later realized `str.format` will take `*args` ## Are there changes in behavior for the user? no ## Is it a substantial burden for the maintainers to support this? no benchmark ```python import timeit import itertools _FORMAT_PATH = "{0}/{1}".format path = "lolonglonglonglonglonglongng/path/to/a/file" print( timeit.timeit( 'itertools.accumulate(path.split("/"), _FORMAT_PATH)', globals=globals() ) ) print( timeit.timeit( 'itertools.accumulate(path.split("/"), lambda x, y: f"{x}/{y}")', globals=globals(), ) ) ``` Co-authored-by: J. Nick Koston <[email protected]>

aiohttp 3.10 and more specifically aio-libs/aiohttp#7944 changed the way cookies are handled. Previously, the domain did not have to be part of the cookie jar for a cookie to be used. As this was changed in 3.10, we now need to pass in the response URL when updating the cookie jar, otherwise the domain will not be part of the stored cookie, and consequently will not be used for the next request. Fixes home-assistant/core#123310.

aio-libs#7583

e91ec9d

xiangxli requested review from webknjaz and asvetlov as code owners December 4, 2023 13:09

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Dec 4, 2023

[pre-commit.ci] auto fixes from pre-commit.com hooks

d901fcf

for more information, see https://pre-commit.ci

xiangxli mentioned this pull request Dec 4, 2023

Implement filter_cookies() with domain-matching and path-matching #7777

Closed

5 tasks

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

tests/test_cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

xiangxli and others added 8 commits December 4, 2023 22:51

Update aiohttp/cookiejar.py

4bbf230

Co-authored-by: Sam Bull <[email protected]>

Update aiohttp/cookiejar.py

9a8a869

Co-authored-by: Sam Bull <[email protected]>

Update aiohttp/cookiejar.py

93250eb

Co-authored-by: Sam Bull <[email protected]>

remove print

26deaee

modify to suggetions

48f4efc

[pre-commit.ci] auto fixes from pre-commit.com hooks

5305795

for more information, see https://pre-commit.ci

modify tests to suggestions

4fcbd08

[pre-commit.ci] auto fixes from pre-commit.com hooks

b176af3

for more information, see https://pre-commit.ci

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

tests/test_cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

tests/test_cookiejar.py Show resolved Hide resolved

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Show resolved Hide resolved

xiangxli and others added 3 commits December 5, 2023 01:56

Update aiohttp/cookiejar.py

02b6baa

Co-authored-by: Sam Bull <[email protected]>

modify tests to suggestions

c7a0d3a

[pre-commit.ci] auto fixes from pre-commit.com hooks

72c88db

for more information, see https://pre-commit.ci

Dreamsorcerer reviewed Dec 4, 2023

View reviewed changes

aiohttp/cookiejar.py Show resolved Hide resolved

Dreamsorcerer reviewed Jan 20, 2024

View reviewed changes

tests/test_cookiejar.py Outdated Show resolved Hide resolved

Update tests/test_cookiejar.py

9ff4082

Dreamsorcerer reviewed Jan 20, 2024

View reviewed changes

CHANGES/7583.feature Outdated Show resolved Hide resolved

Update CHANGES/7583.feature

6adae74

Dreamsorcerer and others added 9 commits January 20, 2024 16:24

Optimise by handling IP outside of loop

84b772e

Also move shared cookie out of loop

5b56087

Merge branch 'master' into master

ded0338

Update test for coverage

cd820e8

Remove now redundant check

8ec5db3

Replace redundant path match

2f6f730

Update cookiejar.py

1cfcd44

[pre-commit.ci] auto fixes from pre-commit.com hooks

8557e39

for more information, see https://pre-commit.ci

Update test_cookiejar.py

ac55d64

Dreamsorcerer merged commit 54ceb6f into aio-libs:master Jan 21, 2024
30 of 34 checks passed

Dreamsorcerer mentioned this pull request Jan 21, 2024

Implement filter_cookies() with domain-matching and path-matching #8046

Merged

bdraco mentioned this pull request Jul 25, 2024

Small speed up to cookiejar filter_cookies #8535

Merged

patchback bot mentioned this pull request Jul 25, 2024

[PR #8535/7108d646 backport][3.10] Small speed up to cookiejar filter_cookies #8537

Merged

slovdahl mentioned this pull request Aug 16, 2024

Fix storing normalized cookie with aiohttp 3.10 Drafteed/pybravia#28

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement filter_cookies() with domain-matching and path-matching #7944

Implement filter_cookies() with domain-matching and path-matching #7944

xiangxli commented Dec 4, 2023

xiangxli commented Dec 4, 2023

codecov bot commented Jan 20, 2024 •

edited

Loading

Dreamsorcerer commented Jan 20, 2024

bdraco commented Jan 20, 2024

bdraco commented Jan 20, 2024

bdraco commented Jan 20, 2024

bdraco commented Jan 20, 2024

bdraco commented Jan 20, 2024

patchback bot commented Jan 21, 2024 •

edited

Loading

Implement filter_cookies() with domain-matching and path-matching #7944

Implement filter_cookies() with domain-matching and path-matching #7944

Conversation

xiangxli commented Dec 4, 2023

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

xiangxli commented Dec 4, 2023

codecov bot commented Jan 20, 2024 • edited Loading

Codecov Report

Dreamsorcerer commented Jan 20, 2024

bdraco commented Jan 20, 2024

bdraco commented Jan 20, 2024

bdraco commented Jan 20, 2024

bdraco commented Jan 20, 2024

bdraco commented Jan 20, 2024

patchback bot commented Jan 21, 2024 • edited Loading

Backport to 3.10: 💔 cherry-picking failed — conflicts found

Backporting merged PR #7944 into master

codecov bot commented Jan 20, 2024 •

edited

Loading

patchback bot commented Jan 21, 2024 •

edited

Loading